-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
COMPAT: Consider Python 2.x tarfiles file-like #16533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f17a20c
to
a9c9365
Compare
Codecov Report
@@ Coverage Diff @@
## master #16533 +/- ##
==========================================
- Coverage 90.79% 90.79% -0.01%
==========================================
Files 161 161
Lines 51063 51075 +12
==========================================
+ Hits 46365 46371 +6
- Misses 4698 4704 +6
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16533 +/- ##
==========================================
+ Coverage 90.75% 90.75% +<.01%
==========================================
Files 161 161
Lines 51073 51081 +8
==========================================
+ Hits 46350 46360 +10
+ Misses 4723 4721 -2
Continue to review full report at Codecov.
|
pandas/io/parsers.py
Outdated
if is_file_like(f): | ||
next_attr = "__next__" if PY3 else "next" | ||
|
||
if engine != "c" and not hasattr(f, next_attr): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a comment of why you are doing this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -702,6 +702,7 @@ def pxd(name): | |||
'parser/data/*.gz', | |||
'parser/data/*.bz2', | |||
'parser/data/*.txt', | |||
'parser/data/*.tar', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add with .tar.gz
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant test with .tar.gz
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. minor comments.
a9c9365
to
6dd7837
Compare
pandas/io/parsers.py
Outdated
# explicitly calls "next(...)" when iterating through | ||
# such an object, meaning it needs to have that attribute. | ||
if engine != "c" and not hasattr(f, next_attr): | ||
msg = ("The 'python' engine can not iterate " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this block seems untested. Are you able to add some, or is it a hassle to hit it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was meaning to do that eventually. Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. couple of comments.
@@ -702,6 +702,7 @@ def pxd(name): | |||
'parser/data/*.gz', | |||
'parser/data/*.bz2', | |||
'parser/data/*.txt', | |||
'parser/data/*.tar', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant test with .tar.gz
as well.
0df7b2c
to
e236ba5
Compare
lgtm. ping on green. |
e236ba5
to
8611b79
Compare
@jreback : Green all around and ready to merge. |
pandas/io/parsers.py
Outdated
else: | ||
engine = "c" | ||
msg += " Falling back to the 'c' engine." | ||
warnings.warn(msg, ParserWarning, stacklevel=5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appears this isn't being hit by test coverage. Should the warning be checked? Can this actually be hit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. This actually can't be hit (engine = python
means the engine was specified). Removing this block.
0526fd6
to
7a5fcd3
Compare
pandas/io/parsers.py
Outdated
@@ -801,6 +803,24 @@ def _get_options_with_defaults(self, engine): | |||
|
|||
return options | |||
|
|||
@staticmethod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this a staticmethod?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because self.
is not used. I pass in the engine
and f
attributes as parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right but no other methods are like this. you can make it a free function if you want, but prob just make it a method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Done.
7a5fcd3
to
08efe9c
Compare
Tarfile.ExFileObject has no "next" method in Python 2.x, making it an invalid file-like object in read_csv. However, they can be read in just fine, meaning our check is too strict for file-like. This commit relaxes the check to just look for "__iter__". Closes pandas-devgh-16530.
08efe9c
to
7c59fc9
Compare
thanks! |
(cherry picked from commit e0a127a)
(cherry picked from commit e0a127a)
Tarfile.ExFileObject
has nonext
method in Python 2.x, making it an invalid file-like object inread_csv
. However, they can be read in just fine, meaning our check is too strict for file-like. This commit relaxesthe check to just look for the
__iter__
attribute.Closes #16530.